Add sorting of all groups and subgroups, recursively #2666
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Siedlerchr
added
the
status: ready-for-review
29 tasks
tobiasdiez
approved these changes
Mar 18, 2017
There was a problem hiding this comment.
The code looks good. I would, however, argue that "sort subgroups" should suffice - if you want to sort the whole tree then just right click the root. And maybe add "recursively" to the menu text to make it a bit clearer.
By the way, we should discuss at some point which other menu entries make sense and which we should remove instead of reimplementing them (e.g. I don't see a point in the move actions).
| @FXML private TreeTableColumn<GroupNodeViewModel, GroupNodeViewModel> disclosureNodeColumn; | ||
| @FXML private CustomTextField searchField; | ||
| @FXML | ||
| private TreeTableView<GroupNodeViewModel> groupTree; |
There was a problem hiding this comment.
Please leave the formatting as it is right now. It is kind of a JavaFX-thing that normally the @FXML tag is on the same line (and I like it that way because otherwise the private fields declaration takes more space)
|
Yeah I will change that, however the fxml tag are not in the formatting yet
I will try to change that as well
Am 19.03.2017 12:07 vorm. schrieb "Tobias Diez" <notifications@github.com>:
*@tobiasdiez* approved this pull request.
The code looks good. I would, however, argue that "sort subgroups" should
suffice - if you want to sort the whole tree then just right click the
root. And maybe add "recursively" to the menu text to make it a bit clearer.
By the way, we should discuss at some point which other menu entries make
sense and which we should remove instead of reimplementing them (e.g. I
don't see a point in the move actions).
------------------------------
In src/main/java/org/jabref/gui/groups/GroupTreeController.java
<#2666 (comment)>:
@@ -43,14 +43,21 @@
private static final Log LOGGER =
LogFactory.getLog(GroupTreeController.class);
- @FXML private TreeTableView<GroupNodeViewModel> groupTree;
- @FXML private TreeTableColumn<GroupNodeViewModel,
GroupNodeViewModel> mainColumn;
- @FXML private TreeTableColumn<GroupNodeViewModel,
GroupNodeViewModel> numberColumn;
- @FXML private TreeTableColumn<GroupNodeViewModel,
GroupNodeViewModel> disclosureNodeColumn;
- @FXML private CustomTextField searchField;
+ @FXML
+ private TreeTableView<GroupNodeViewModel> groupTree;
Please leave the formatting as it is right now. It is kind of a
JavaFX-thing that normally the @FXML <https://github.com/FXML> tag is on
the same line (and I like it that way because otherwise the private fields
declaration takes more space)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2666 (review)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AATi5NTWFJ6ef70b1vsgEXCg-s3TNYEBks5rnGOVgaJpZM4Mhd6n>
.
|
tobiasdiez
removed
the
status: ready-for-review
* upstream/master: Localization: General: French: Translation of new entries Localization: Menu: French: Translation of an entry (#2685) Fix #2680 and fix #2667: Swing errors are catched properly and without freezing (#2681) Do not log AND throw Replace misleading error message for fetcher connection error Document CrossRef test Fix subtitle detection for CrossRef fetcher Revert "Invoke LogMessages.add in JavaFX thread" Use global user agent Update mockito from 2.7.17 to 2.7.18 Move GuiAppender to GUI package Invoke LogMessages.add in JavaFX thread [WIP] Put the PDFAnnotationImporter under Test, enhance FileAnnotationTab (#2640) Fix for "Paying off technical debt: almost all utility classes have a private constructor now." (#2672) Revert "Paying off technical debt: almost all utility classes have a private constructor now. (#2649)" (#2670) Paying off technical debt: almost all utility classes have a private constructor now. (#2649) Changed codeformatting for better fxml annotation (#2668) Disalbe Google Scholar tests on all CI environments (#2654) Fix JSONException in Crossref fetcher as mentioned in #2442
|
@tobiasdiez Changed localization, but I dunno how to display the menu entry only when the root nod eis selected? |
|
In However, I would completely leave of the |
|
I thin we should stuck with simply having the sort groups recursively. Then we could omit the explicit chek for root node |
* upstream/master: (35 commits) Update antlr from 4.6 to 4.7 Fix build fix ID consideration in DuplicateCheck Add ArXiv identifier batch lookup (#2710) Update mockito from 2.7.19 to 2.7.21 More defensive identifier list #2708 Revert "Add more identifier field names #2708" Add more identifier field names #2708 Consider entries as equal if their DOI matches #2708 Imports Imports Move duplicate detection to logic Reuse edit distance class Refactoring EntryTypeDialog Fetching Autogenerates BibTeX Key (#2709) Add changelog entry Increase permitted size of StringUtil Make sure that JavaFx shuts down in case another JabRef instance is already open Remove obsolete localization strings Hide context menu before group edit/add (probably a JavaFX vs Swing problem) ... # Conflicts: # src/main/java/org/jabref/gui/groups/GroupTreeController.java # src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
Siedlerchr
added a commit
that referenced
this pull request
Apr 13, 2017
* upstream/master: (39 commits) Fix fetcher test Allow failures for fetcher test (#2730) Use JabRefExecutor service Move DOI fetching to separate thread #2682 Remove gui dependency in logic (#2726) Fixed freeze on Mac OS X when creating/editing groups (#2727) Only ask once if telemetry data should be collected Update wiremock from 2.5.1 to 2.6.0 Update mockito-core from 2.7.21 to 2.7.22 Update log4j to latest version Azure test (#2724) Fix build Move expand filename to FileUtil Unicode conversion bibtexkey (#2720) Add sorting of all groups and subgroups, recursively (#2666) Only check capitalization of note and howpublished fields if they start with a word character Remove overhauled @author tag Implement #1359: collect telemetry (#2283) Add licenses of new dependencies Fix cssStyleHelper warnings ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements some features from #2599
Recursively sort all groups or selected groups
Replaced Screenshot
Sometimes the sorting is not always directly correct visible after sorting, This is somehow an update problem of the gui I think. When you switch db back and forth it's correctly there.
- [ ] Change in CHANGELOG.md described- [ ] Tests created for changes- [ ] Check documentation status (Issue created for outdated help page at help.jabref.org?)gradle localizationUpdate?